-
Notifications
You must be signed in to change notification settings - Fork 154
docs(all): make docs more coherent #387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tools-typescript into docs/general_improv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! :D Just minor comments, but looking forward to merging this one
Co-authored-by: Sara Gerion <[email protected]>
Co-authored-by: Sara Gerion <[email protected]>
Co-authored-by: Sara Gerion <[email protected]>
Co-authored-by: Sara Gerion <[email protected]>
Thanks @saragerion, I've found&replaced all other occurrences to |
Co-authored-by: Florian Chazal <[email protected]>
Co-authored-by: Florian Chazal <[email protected]>
Co-authored-by: Florian Chazal <[email protected]>
Co-authored-by: Florian Chazal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve. I have some nit and minor comments. But I prefer to have this merged than getting things 100% perfect.
@dreamorosi Appreciate your effort on this. I can see that you really read through and caught all minor details.
@@ -74,7 +74,7 @@ For a **complete list** of supported environment variables, refer to [this secti | |||
Environment: | |||
Variables: | |||
LOG_LEVEL: WARN | |||
POWERTOOLS_SERVICE_NAME: shopping-cart-api | |||
POWERTOOLS_SERVICE_NAME: serverlessAirline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(minor) There are a few more "shopping-cart-api" left in the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I've missed those. I've now removed all of the ones in the serviceName
but left the ones in the function name
@logger.injectLambdaContext() | ||
public handler() { | ||
public async handler(_event: any, _context: any): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
Co-authored-by: ijemmy <[email protected]>
Co-authored-by: ijemmy <[email protected]>
…owertools-typescript into docs/general_improv
Description of your changes
This PR includes miscellaneous changes derived from early feedbacks & internal discussion.
Some changes include (but not limited to):
->Tracer
new Tracer()
)LambdaInterface
& async signature in examplesHow to verify this change
Run docs locally
npm run docs-runLocalDocker
& head to http://localhost:8000Related issues, RFCs
#372
PR status
Is this ready for review?: YES
Is it a breaking change?: NO
Checklist
Breaking change checklist
N/A
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.